-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[feat][client] PIP-446: Support Native OpenTelemetry Tracing in Pulsar Java Client #24873
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
@asafm Do you have a chance to review this PR? |
|
@lhotari yes. This was the only comment I saw. |
poorbarcode
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have one suggestion, it should be useful to determine client-side publish latency issue
- Should we add more events for producers, to let us to record more spans? Such as follows
users call send->cache message with message containerpublish message to broker->receive broker response
|
|
||
| @Override | ||
| public void close() { | ||
| // No cleanup needed - spans are attached to messages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // No cleanup needed - spans are attached to messages | |
| // Producer will fail pending messages when it being closed, which will trigger the `onSendAcknowledgement` events |
| @Override | ||
| public Message beforeSend(Producer producer, Message message) { | ||
| // Initialize tracer from producer on first call | ||
| initializeIfNeeded(producer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we treat the first and last parts of a chunked message differently?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please ignore this comment channel, because sendComplete already covered the case here: https://github.com/apache/pulsar/blob/v4.1.2/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java#L1372-L1381
Motivation
Implementation of PIP-446
Documentation
docdoc-requireddoc-not-neededdoc-complete